Use Libtask_jll #76
Use Libtask_jll #76
Conversation
| - name: CompatHelper.main() | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| COMPATHELPER_PRIV: ${{ secrets.COMPATHELPER_PRIV }} |
There was a problem hiding this comment.
CI tests will only run if an SSH key is added (see https://github.com/JuliaRegistries/CompatHelper.jl#12-set-up-the-ssh-deploy-key-optional).
| @@ -3,16 +3,16 @@ uuid = "6f1fad26-d15e-5dc8-ae53-837a1d7b8c9f" | |||
| license = "MIT" | |||
| desc = "C shim for task copying in Turing" | |||
| repo = "https://github.com/TuringLang/Libtask.jl.git" | |||
| version = "0.4.2" | |||
| version = "0.5.0" | |||
There was a problem hiding this comment.
This allows us to backport and release bugfixes for Julia < 1.3 in Libtask 0.4.* if needed.
| function __init__() | ||
| check_deps() | ||
| end | ||
| export CTask, consume, produce, TArray, tzeros, tfill, TRef |
There was a problem hiding this comment.
There is no need to reexport Base.get, so I removed it. I'm wondering if consume and produce should be removed from the exports as well since their names are quite generic.
|
There are some reproducible test errors on Windows32 with Julia 1.5 (and test errors on nightly since apparently internals of Task were changed). Can they be fixed? |
|
So where the removed files (c source, makefile, build script, etc) are placed now? I did a simple search and failed to find them... |
|
The intention was to not keep them in the repo here but to move them to Yggdrasil. That would keep everything together in one place and make it easier to update and rebuild the library, without making PRs to two repositories. BTW there are efforts to link libraries against Julia on Yggdrasil, and it seems there's already official support for Julia 1.4 (but not all OS) and 1.5. So at some point we might be able to get rid of the different libraries and the checks in Libtask.jl, and instead only use a Julia-versioned |
I think it would be better to move the relevant files to Yggdrasil and build a new version of Libtask_jll before merging this PR, what do you think?
Great, the build process could be dramatically simplified if it is true :) |
Sounds reasonable 👍 At the same time, we should maybe build Julia 1.4 and 1.5 specific versions as well? I'm wondering if this could fix the test issues on Windows 32bit with Julia 1.5. |
|
@KDr2 let's get this PR merged asap. |
|
We have to wait until the libraries are available in the registry. |
How many versions of the JLL should we build in Yggdrasil? At least 3 I think? As discussed in JuliaPackaging/Yggdrasil#2087, we have to file a PR for each version and wait until all of them are available in the registry。 |
|
Yes, exactly. I wanted to test this PR locally with the library for Julia 1.3 first before opening the other PRs. However, the library is not available in the general registry since automerging failed: JuliaRegistries/General#24916 |
|
The new library is still not registered but tests passed locally: JuliaRegistries/General#24916 |
|
It seems to work 🎉 The pre-built Libtask_jll is now available for Julia 1.3 (Libtask_jll 0.4.0), Julia 1.4 (Libtask_jll 0.4.1), and Julia 1.5 (Libtask_jll 0.4.2). In the tests the correct version of Libtask_jll is installed for these Julia versions. Tests for Julia nightly fail since there is no compatible version of Libtask_jll available. |
Maybe consider falling back to most recent Libtask_jll release (e.g. 0.4.2) for the nightly build? |
I think Pkg does not allow this. And actually I think it is reasonable since otherwise we basically create "unbounded" Libtask_jll versions and newer Julia versions might just fall back to completely broken libraries. As far as I know, however, it would be possible to add the Libtask_jll repo manually which then circumvents the checks in the registry. At least that was the case in my local tests - Pkg then just prints a warning about incompatible Julia versions but still installs the latest version. We could add this to the CI tests? |
Makes sense since we'll know when breaking changes are introduced into the nightly build. Also, maybe consider adding a line in the README file, informing potential nightly Julia users how to install manually? |
|
OK, tests run now on Julia nightly as well with the latest version of Libtask_jll but cause a Julia segfault (not too surprising, they were already broken on master since base Julia removed fields of Task etc.). I also added a section to the README. |
|
Excellent work, many thanks @devmotion! |
|
@yebai Can you fix #76 (comment) and add a secret for CompatHelper? Or maybe that has been done already? |
This PR fixes #75 by removing the custom build process with BinaryProvider and instead using the prebuilt libraries in Libtask_jll.
This change requires Julia >= 1.3 but I assume this might be fine since recent versions of Turing only support Julia >= 1.3.